perf(lsp): resolve code lenses eagerly to fix ~25s document-open stall - #13
Merged
Merged
Conversation
…chmark - TestNewRequestParseMemoizesPerPath: same path → identical *ParseResult pointer, proving at-most-once parse per request - TestNewRequestParseKeyedByCleanPath: "dir/./file.ridl" and "dir/file.ridl" share the same memo slot - BenchmarkCodeLensEager: 40-importer workspace baseline — ~11.9 ms/op on Apple M4 Pro (arm64)
…+ ParseAST equivalence tests
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Opening a RIDL document fired ~17
codeLens/resolvecalls (one per enum/struct/error), and each one re-parsed the entire workspace — and every parse recursively re-parsed the whole import graph twice (the schema build plus a second upstream pass). On a real project that was ~1.5s per resolve, ~25s of CPU per document open, serialized behind the editor. This makes CodeLens compute its reference counts eagerly in one pass and parse each file once per request via an AST-only parser, so a document open is a single workspace traversal instead of 17 full re-scans.What changed
CodeLensnow returns fully-resolved lenses (reference-countCommandalready set);CodeLensResolveis a passthrough andResolveProvideris advertised asfalse. This removes the per-lens resolve storm and the staleness surface of a per-version resolve memo — the lens list is a snapshot of oneCodeLensrequest, which the client re-issues on change.ParseAST(new). An overlay-aware, AST-only parse: the upstream AST head only, skippingbuildPartialSchema's recursive import parse and the secondridl.NewParserschema pass.Rootis structurally identical to a fullParse;Schemais a non-nil empty value; a partial root is still returned on parse error (mid-edit keeps working).parseFninjection. Definition/reference resolution is parametrized by a parse function. TheServermethods stay as adapters binding the existing full-parse path, so every non-CodeLens handler is unchanged; onlyCodeLensinjects a request-scoped memoized AST parser (keyed byfilepath.Clean, overlays snapshotted once, open-buffer results reused).ctx.Err()and never leaks a partial or empty result as success.This is PR1 of two. PR2 (separate) adds a session-level parse cache +
**/*.ridlfile watchers to speed references/rename/diagnostics across edits; it is intentionally out of scope here.Test plan
New tests:
ParseASTreturns Root + non-nil Schema and skips import resolution;ParseASTvsParseRoot structural equivalence; request-scoped memo parses each path once and is keyed by cleaned path; eager CodeLens builds the show-references command with correct counts for same-file, imported-type, and error references;CodeLensResolvepassthrough; cancelled-context CodeLens returns an error.Review notes
ParseAST/eager-CodeLens shape), then the implementation diff went through a Claude per-task + whole-branch review (which caught theResolveProvider/passthrough mismatch) and a Codex adversarial diff review (which caught the cancellation leak). Both are fixed.setupServerbecause it only takes*testing.T(make ittesting.TBas cleanup); upstream parser execution itself is not interruptible mid-parse — cancellation is cooperative between file parses, which is sufficient here since the loop stops before each parse and before returning.